Skip to content

Conversation

@mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Jun 27, 2016

This adds support for storing signatures in Atomic Registry, assuming server-side support per (merged) openshift/origin#8371 and (currently unmerged) openshift/origin#9181 .

(Marked as WIP because the server side is missing, and because this currently requires a cluster-wide “update signatures” privilege, which seems impractical, so future changes of the API cannot be ruled out. Also, this is currently done as 2 commits to preserve the 2 possible ways to upload signatures, after the decisions are final they should be squashed.)

@philips
Copy link

philips commented Jun 28, 2016

cc @dgonyeo @ecordell @gtank

@mtrmac mtrmac force-pushed the openshift-native-signatures branch from 0c44ed5 to 653fd7a Compare June 28, 2016 16:14
// SignedClaims map[string]string `json:"signedClaims,omitempty"`
// Created *unversioned.Time `json:"created,omitempty"`
// IssuedBy SignatureIssuer `json:"issuedBy,omitempty"`
// IssuedTo SignatureSubject `json:"issuedTo,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's up with all of these commented out fields? Something to be used in the future?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those fields exist in the upstream OpenShift objects, and we do not use them (In particular these should be filled in by the server, to have something to display in an UI, so we want to neither fill them in (the server will override that) nor read them (we want to make independent policy decisions without trusting the server)).

This ~ only acknowledges their existence, and unlike uncommented fields we do not need to define the used types etc.

@mtrmac mtrmac force-pushed the openshift-native-signatures branch 4 times, most recently from b3f068a to ea18e1e Compare July 4, 2016 10:30
@mtrmac mtrmac force-pushed the openshift-native-signatures branch 3 times, most recently from fc7b499 to a2e8c82 Compare July 18, 2016 12:46
@mtrmac mtrmac force-pushed the openshift-native-signatures branch 2 times, most recently from 27c8657 to 3ebb1e8 Compare July 19, 2016 13:33
@mtrmac mtrmac force-pushed the openshift-native-signatures branch 2 times, most recently from 321ab65 to 9a6b762 Compare August 1, 2016 17:50
@mtrmac mtrmac changed the title WIP OpenShift native signatures OpenShift native signatures Aug 1, 2016
@mtrmac
Copy link
Collaborator Author

mtrmac commented Aug 1, 2016

Updated against the final merged API from openshift/origin#9181 .

I'm afraid this still requires a cluster-wide signature privilege, but it does work and it is hopefully already useful.

@mtrmac
Copy link
Collaborator Author

mtrmac commented Aug 1, 2016

See also containers/skopeo#93 for the necessary copy.go update. (Because we don’t yet have tests of signature handling, not merging that doesn’t show up as a test failure. Tests exist in https://github.com/mtrmac/skopeo/tree/integrate-all-the-things , and are ~blocked on really finishing https://github.com/mtrmac/skopeo/tree/verify-on-pull ).

Read imagestreamimage to read signatures.

Add signatures one by one.  The signatures are shared for a single image
across any namespaces, so we don’t delete (others’) signatures.

Note that this currently requires a cluster-wide signature write
privilege, system:image-signer!

A minimal configuration to make this work, though leaving permissions
wide open, is
> oadm policy add-cluster-role-to-group system:image-signer system:authenticated

Signed-off-by: Miloslav Trmač <[email protected]>
@runcom
Copy link
Member

runcom commented Aug 1, 2016

lgtm

Approved with PullApprove

@mtrmac
Copy link
Collaborator Author

mtrmac commented Aug 1, 2016

Because we don’t yet have tests of signature handling, not merging that doesn’t show up as a test failure.

Actually it does, because even PutSignatures(empty_slice) triggers the ordering check. Anyway, containers/skopeo#93 fixes that.

@mtrmac
Copy link
Collaborator Author

mtrmac commented Aug 1, 2016

👍

Approved with PullApprove

@mtrmac mtrmac merged commit 53ce7ca into containers:master Aug 1, 2016
@mtrmac mtrmac deleted the openshift-native-signatures branch August 1, 2016 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants